Skip to content

84 possible refactor - #92

Open
AymanBx wants to merge 23 commits into
mainfrom
84-possible-refactor
Open

84 possible refactor#92
AymanBx wants to merge 23 commits into
mainfrom
84-possible-refactor

Conversation

@AymanBx

@AymanBx AymanBx commented Jun 10, 2026

Copy link
Copy Markdown
Contributor
  • Runner has run method that is called by task
    • run logic moved to runner
    • Runner object has param dict as an attribute
    • runner now creates a dict called response info that includes token count and stop reason in addition to response
  • Logger class
  • Logger objects will either be initialized in bench.run or task.run
  • Logger object passed between bench, task and runner
    • Each will log its info, then runner logs interaction (each logging step has a method)
  • Logging file structure modified
    • Bench has its own run_id that is included in task's run_id
  • Bench_score works with the new structure.
    • Collate option removed
  • Task has score method renamed to score_prompt
  • Added demo files to demonstrate the new changes

@AymanBx
AymanBx requested a review from brownsarahm June 10, 2026 18:41
@AymanBx AymanBx linked an issue Jun 10, 2026 that may be closed by this pull request
This was linked to issues Jun 18, 2026
@@ -1,4 +1,4 @@
runner_type: ollama
model:
models:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enable specifying the parameters under here

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

combine this with runnery.yml under 'model_params' key and then pass that whole sub-dictionary to the model

Comment thread benchtools/benchmark.py Outdated
for task_dict in task_list:
tasks.append(Task.from_dict(task_dict,source_path=benchmark_path))

runners = Bench.load_runners(benchmark_path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call the from_file on benchlist instead and in the bench constructor

@AymanBx
AymanBx marked this pull request as ready for review July 13, 2026 06:26

@brownsarahm brownsarahm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor chagnes + please check docs for necessary updates

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why this change?

Comment thread benchtools/benchmark.py
The path to the benchmark folder
'''
current_dir = os.getcwd()
os.chdir(benchmark_path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a comment why thi sis being done this way

Comment thread benchtools/benchmark.py
os.system("git branch -m main")
except:
print("git might not be initialized in your system. Please run \"git init . \" when setup")
# Get python gitignore template and create .gitignore

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change "python" here to package

Comment thread benchtools/benchmark.py


def run(self, runner=BenchRunner(), log_dir=None, score=False):
def run(self, runner=BenchRunner(), log_dir=None):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is scoring no longer possible while running? I know we were removing from the file, but why not allow it?

Comment thread benchtools/benchmark.py
Comment on lines +326 to +327
import subprocess
from datetime import datetime

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all imports at the top of the file

Comment thread benchtools/benchmark.py
Comment on lines +332 to +357
out = subprocess.run(
"git diff",
shell=True,
capture_output=True,
text=True)
if out.stdout.strip() != '':
# Get current date and time
string_timestamp = datetime.now().strftime("%Y-%m-%d %H:%M:%S")

subprocess.run(
f"git add -A",
shell=True,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL)
subprocess.run(
f"git commit -m \"Bench run commit: {string_timestamp}\"",
shell=True,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL)

commit = subprocess.run(
"git log -1 --oneline",
shell=True,
capture_output=True,
text=True).stdout.strip()
commit_hash, commit_message = commit.split(' ', 1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is all of this for? add comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

possible refactor add token counts improve log structure

2 participants